refactor: parallelize docs rendering and Shiki setup#2382
refactor: parallelize docs rendering and Shiki setup#2382trivikr wants to merge 6 commits intonpmx-dev:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe PR parallelises documentation rendering by converting several sequential await loops into concurrent Promise.all operations. In server/utils/docs/render.ts it renders kind sections, symbols, signatures, descriptions and JSDoc tags in parallel. server/utils/docs/text.ts now highlights all fenced code blocks concurrently and restores placeholders by index. server/utils/shiki.ts introduces a shared in‑flight highlighter promise to avoid duplicate initialisation and clears it on failure. Tests add createFunctionSymbol/createInterfaceSymbol helpers and new suites asserting stable section and symbol ordering despite parallel rendering. Suggested reviewers
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
server/utils/docs/text.ts (1)
138-141: Consider usingforEachfor type-safe iteration.The non-null assertion
highlightedCodeBlocks[i]!bypasses TypeScript's type checking. While the loop bounds guarantee valid indices, aforEachcallback would be strictly type-safe without the assertion.♻️ Suggested refactor
- for (let i = 0; i < highlightedCodeBlocks.length; i++) { - const highlighted = highlightedCodeBlocks[i]! - result = result.replace(`__CODE_BLOCK_${i}__`, highlighted) - } + highlightedCodeBlocks.forEach((highlighted, i) => { + result = result.replace(`__CODE_BLOCK_${i}__`, highlighted) + })As per coding guidelines: "Ensure you write strictly type-safe code, for example by ensuring you always check when accessing an array value by index".
server/utils/docs/render.ts (1)
138-161: Destructured array elements may benulldespite conditional guards.The variables
highlightedSignature,renderedDescription, andrenderedJsDocTagsare typed asstring | null. The conditionals at lines 144, 154, and 159 mirror the conditions used to create the promises, but TypeScript cannot correlate these checks.The runtime behaviour is correct, but for stricter type safety, consider using truthiness checks directly on the resolved values (which you already do at lines 154 and 159) or adding explicit null checks.
♻️ Minor type-safe adjustment for line 145
if (signatures.length > 0) { - lines.push(`<div class="docs-signature">${highlightedSignature}</div>`) + lines.push(`<div class="docs-signature">${highlightedSignature ?? ''}</div>`)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b64ac82b-b369-41a2-b097-ee16c3a7f693
📒 Files selected for processing (4)
server/utils/docs/render.tsserver/utils/docs/text.tsserver/utils/shiki.tstest/unit/server/utils/docs/render.spec.ts
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bd79fef8-b292-4e38-9c0d-e632936bf7fc
📒 Files selected for processing (2)
server/utils/docs/render.tsserver/utils/docs/text.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- server/utils/docs/render.ts
- Render doc sections, symbols, markdown blocks, and examples concurrently - Cache in-flight Shiki highlighter initialization to avoid duplicate work - Add ordering tests for parallel docs rendering
trivikr
force-pushed
the
doc-gen-parallel
branch
from
92d69f3 to
e70b385
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a0e3e0ce-a6cc-407f-becf-2035078899e8
📒 Files selected for processing (4)
server/utils/docs/render.tsserver/utils/docs/text.tsserver/utils/shiki.tstest/unit/server/utils/docs/render.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/unit/server/utils/docs/render.spec.ts
| const examplePromises = examples.map(async example => { | ||
| if (!example.doc) return '' | ||
| const langMatch = example.doc.match(/```(\w+)?/) | ||
| const lang = langMatch?.[1] || 'typescript' | ||
| const code = example.doc.replace(/```\w*\n?/g, '').trim() | ||
| return highlightCodeBlock(code, lang) |
There was a problem hiding this comment.
Handle hyphenated fenced languages in @example blocks.
These regexes only consume \w, so a valid fence like ```glimmer-ts is parsed as language glimmer and leaves -ts at the start of the code body. That breaks example rendering for some Shiki languages now loaded in server/utils/shiki.ts, instead of falling back cleanly.
Suggested fix
const examplePromises = examples.map(async example => {
- if (!example.doc) return ''
- const langMatch = example.doc.match(/```(\w+)?/)
- const lang = langMatch?.[1] || 'typescript'
- const code = example.doc.replace(/```\w*\n?/g, '').trim()
- return highlightCodeBlock(code, lang)
+ const doc = example.doc?.trim()
+ if (!doc) return ''
+
+ const fencedMatch = doc.match(
+ /^```[ \t]*([^\s`]*)[ \t]*(?:\r\n|\r|\n)([\s\S]*?)(?:\r\n|\r|\n)?```$/,
+ )
+ if (!fencedMatch) {
+ return highlightCodeBlock(doc, 'typescript')
+ }
+
+ const [, rawLang = '', code = ''] = fencedMatch
+ return highlightCodeBlock(code.trim(), rawLang || 'typescript')
})3 participants
🔗 Linked issue
N/A
🧭 Context
Docs generation was doing more sequential work than necessary in the API renderer. We were awaiting kind sections one at a time, then symbols one at a time within each section, and each symbol also awaited syntax highlighting and markdown rendering serially.
For packages with large API surfaces, that made total render time closer to the sum of all individual steps instead of allowing independent work to overlap.
📚 Description
This PR parallelizes the independent async work in the docs rendering pipeline while preserving stable output order.
Changes:
renderDocNodesThe rendered HTML order is unchanged:
KIND_DISPLAY_ORDER